Reviewed 48 of 48 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions

android/settings.gradle.kts line 26 at r1 (raw file):

    ":lib:resource",
    ":lib:shared",
    ":lib:shared-compose",

Let's talk about this name internally, I think we should align it how it fits in our gradle module restructuring.

android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/InAppNotification.kt line 1 at r1 (raw file):

package net.mullvad.mullvadvpn.lib.shared

Should probably not be in shared but rather the lib/model

android/lib/shared-compose/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/compose/test/ComposeTestTagConstants.kt line 1 at r1 (raw file):

package net.mullvad.mullvadvpn.lib.shared.compose.test

Not sure where we want these testTags to live, both test and code use them. They are the worst, let's discuss internally when we talk about the rest of the modules.

android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/VersionInfo.kt line 1 at r1 (raw file):

package net.mullvad.mullvadvpn.lib.shared

Possibly should be in domain (depending if it makes sense, it is more part of a repository thing it may still live here)

Reviewable status: 47 of 49 files reviewed, 4 unresolved discussions (waiting on @Rawa)

android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/InAppNotification.kt line 1 at r1 (raw file):

Done.

android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/VersionInfo.kt line 1 at r1 (raw file):

Done.

Reviewed 2 of 2 files at r2, 25 of 25 files at r3, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @kl and @Rawa)

Reviewed 1 of 2 files at r2, 21 of 25 files at r3, 14 of 14 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion

android/lib/tv/src/main/kotlin/net/mullvad/mullvadvpn/lib/tv/NavigationDrawerTv.kt line 114 at r4 (raw file):

                Modifier.fillMaxHeight()
                    .background(brush)
                    .padding(top = 24.dp, bottom = 24.dp, start = 12.dp, end = 12.dp)

Should be represented in the dimens file.

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @kl)

android/lib/tv/src/main/kotlin/net/mullvad/mullvadvpn/lib/tv/NavigationDrawerTv.kt line 117 at r4 (raw file):

                    .selectableGroup()
            ) {
                val animatedPadding = animateDpAsState(if (hasFocus) 16.dp else 12.dp)

ditto

Reviewed 2 of 48 files at r1, 1 of 14 files at r4.
Reviewable status: 47 of 50 files reviewed, 3 unresolved discussions (waiting on @kl, @Pururun, and @Rawa)

android/lib/component/build.gradle.kts line 8 at r4 (raw file):

android {
    namespace = "net.mullvad.mullvadvpn.lib.shared.compose"

As discussed afk, let's instead go with the more explicit lib.ui.component. This should also be reflected in the file structure (lib/ui/component) as well as in package names. This is what we decided (but forgot) in previous package/module discussions: https://linear.app/mullvad/issue/DROID-1651

Code quote:

lib.shared.compose

Reviewable status: 47 of 50 files reviewed, 3 unresolved discussions (waiting on @Pururun and @Rawa)

android/lib/tv/src/main/kotlin/net/mullvad/mullvadvpn/lib/tv/NavigationDrawerTv.kt line 114 at r4 (raw file):

Done.

android/lib/tv/src/main/kotlin/net/mullvad/mullvadvpn/lib/tv/NavigationDrawerTv.kt line 117 at r4 (raw file):

Done.

Reviewable status: 31 of 50 files reviewed, 3 unresolved discussions (waiting on @albin-mullvad, @Pururun, and @Rawa)

android/lib/component/build.gradle.kts line 8 at r4 (raw file):

Done.

Reviewed 2 of 3 files at r5, 11 of 16 files at r6, all commit messages.
Reviewable status: 44 of 50 files reviewed, 4 unresolved discussions (waiting on @albin-mullvad and @Pururun)

android/settings.gradle.kts line 29 at r7 (raw file):

    ":lib:theme",
    ":lib:tv",
    ":lib:ui",

Is it needed? Seemed to work w/o.

android/lib/tv/src/main/kotlin/net/mullvad/mullvadvpn/lib/tv/NavigationDrawerTv.kt line 125 at r7 (raw file):

                val animatedPadding =
                    animateDpAsState(
                        if (hasFocus) Dimens.tvDrawerHeaderStartPadding + 4.dp

maybe tvDrawerHeaderWithFocusStartPadding ?

Reviewed 1 of 16 files at r6.
Reviewable status: 45 of 50 files reviewed, 3 unresolved discussions (waiting on @albin-mullvad, @kl, and @Pururun)

Reviewed 4 of 16 files at r6, 1 of 1 files at r7.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @albin-mullvad and @kl)

Reviewed 3 of 3 files at r8, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @albin-mullvad)

Reviewed 1 of 48 files at r1, 6 of 16 files at r6, 1 of 3 files at r8, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion

android/lib/component/build.gradle.kts line 8 at r4 (raw file):

Nice!

android/settings.gradle.kts line 32 at r8 (raw file):

)

include(":test", ":test:arch", ":test:common", ":test:e2e", ":test:mockapi")

Was this automagically formatted? Are we still missing some automated formatting check?

Reviewable status: 49 of 50 files reviewed, all discussions resolved (waiting on @albin-mullvad and @Rawa)

android/settings.gradle.kts line 32 at r8 (raw file):

It was my IDE autoformatter. I tested this and it looks like ./gradlew ktfmtFormat does not format our gradle scripts. I changed this back manually for now but we probably want to figure out how to enable auto formatting for our gradle build scripts.

Reviewable status: 49 of 50 files reviewed, all discussions resolved (waiting on @Rawa)

android/settings.gradle.kts line 32 at r8 (raw file):

Aha! But doesn't it format both buildSrc and our build.gradle.kts scripts? Could it skip just settings.gradle.kts? 🤔 Any idea whether we can make it format all of those otherwise?

Reviewed 1 of 16 files at r6, 1 of 1 files at r9.
Reviewable status: all files reviewed, 2 unresolved discussions

android/settings.gradle.kts line 13 at r9 (raw file):

rootProject.name = "MullvadVPN"

include(":app", ":service", ":tile")

Revert ⏪

android/gradle/verification-metadata.xml line 200 at r8 (raw file):

            <trusting group="androidx.fragment"/>
            <trusting group="androidx.graphics"/>
            <trusting group="androidx.lifecycle"/>

Remember to put the lockfile update last so we easily can add a signature 🙏

Reviewed 1 of 1 files at r9.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @kl)

Reviewed 10 of 10 files at r10.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @kl)

Reviewed 1 of 10 files at r10.
Reviewable status: complete! all files reviewed, all discussions resolved